[MatrixRTC] Implement MSC4143 slots #5056

Half-Shot · 2025-10-28T12:12:47Z

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
Sign-off given on the changes (see CONTRIBUTING.md).

toger5 · 2025-10-30T14:56:39Z

src/matrixrtc/CallMembership.ts

+            case MembershipKind.RTC:
                return data.rtc_transports[0];
-            case "session":
-                switch (data.focus_active.focus_selection) {
-                    case "multi_sfu":
-                        return data.foci_preferred[0];
-                    case "oldest_membership":
-                        if (CallMembership.equal(this, oldestMembership)) return data.foci_preferred[0];
-                        if (oldestMembership !== undefined) return oldestMembership.getTransport(oldestMembership);
-                        break;
+            case MembershipKind.Session:
+                if (data.focus_active.focus_selection === "oldest_membership") {
+                    // For legacy events we only support "oldest_membership"
+                    if (CallMembership.equal(this, oldestMembership)) return data.foci_preferred[0];
+                    if (oldestMembership !== undefined) return oldestMembership.getTransport(oldestMembership);
                }
+                break;
        }
        return undefined;
    }


In general the changes to the call membership are much more straight forward than what I anticipated. this seems to be the main functional change.

Is there anything else in the code that gets more complicated because of this?

Not really, I only removed things which didn't break other files or ECall / EWeb. I think the conflicts with the other PRs may be high.

toger5 · 2025-10-30T15:04:45Z

src/matrixrtc/MatrixRTCSession.ts

+                const content = e.getContent<RtcMembershipData>();
+                // Ensure the slot ID of the membership matches the state
+                if (content.slot_id !== slotId) {
+                    console.log("Invalid slot ID", content.slot_id, slotId);
+                    return false;
+                }
+                if (content.application.type !== slotDescription.application) {
+                    console.log("Invalid application.type", content.application.type, slotDescription.application);
+                    return false;
+                }
+                return true;


Is this all covered by:

if (!deepCompare(membership.slotDescription, slotDescription))

which is done later?

…or-call

BillCarsonFr

Looks sane, I have some minor comments. It is a bit big of a change to see the precise change of logic but ok for me

BillCarsonFr · 2025-11-07T08:03:24Z

spec/unit/matrixrtc/MembershipManager.spec.ts

    let client: MockClient;
    let room: Room;
-    const focusActive: LivekitFocusSelection = {
+    const focusActive = Object.freeze({


TIL. Looks like this needs some strict mode? How is it defined? or maybe it is default?

BillCarsonFr · 2025-11-07T08:06:58Z

spec/unit/matrixrtc/MembershipManager.spec.ts

+    describe("LegacyMembershipManager", () => {
+        beforeEach(() => {
+            // Provide a default mock that is like the default "non error" server behaviour.
+            (client._unstable_sendDelayedStateEvent as Mock<any>).mockResolvedValue({ delay_id: "id" });


What is the difference with this notation jest.mocked(client._unstable_sendDelayedStateEvent).mockResolvedValue({ delay_id: "id" }); ?
That I think I usually see for that

BillCarsonFr · 2025-11-07T08:10:29Z

src/matrixrtc/membership/common.ts

@@ -0,0 +1,11 @@
+/**


BillCarsonFr · 2025-11-07T08:20:56Z

src/@types/event.ts

    CallNotify = "org.matrix.msc4075.call.notify",
    RTCNotification = "org.matrix.msc4075.rtc.notification",
    RTCDecline = "org.matrix.msc4310.rtc.decline",
+    RTCSlot = "org.matrix.msc4143.rtc.slot",


Sometimes I see that we use new UnstableValue(stable, mcxxx.unstable) and other times not. Any reason not to use it everytime? Combined with EitherAnd and the findIn it looks pretty useful, WDYT?

BillCarsonFr · 2025-11-07T08:22:17Z

src/matrixrtc/membership/legacy.ts

@@ -0,0 +1,121 @@
+import { type IContent } from "../../matrix.ts";


missing ™️

BillCarsonFr · 2025-11-07T09:54:41Z

src/matrixrtc/MatrixRTCSession.ts

-        });
-    }
+    public static getRtcSlot(
+        room: Pick<Room, "getLiveTimeline" | "roomId">,


oh that's new, we need the timelime now. I guess it is the only to access state?

BillCarsonFr · 2025-11-07T09:56:38Z

src/matrixrtc/MatrixRTCSession.ts

-    }
+    public static getRtcSlot(
+        room: Pick<Room, "getLiveTimeline" | "roomId">,
+        slotDescription: SlotDescription,


I recently read the code_style.md and I think it is recommanding to use Optional<X> instead of RtcSlotEventContent | null but it is used almost no where :D

BillCarsonFr · 2025-11-07T10:06:05Z

src/matrixrtc/MatrixRTCSession.ts

+        }
+        if (
+            "type" in slotContent.application === false ||
+            slotContent.application.type !== slotDescription.application


not sure to read correctly but if application.type and slotDescription.application should be the same, maybe we have a naming problem? Can we use slotDescription.applicationType?

BillCarsonFr · 2025-11-07T10:10:55Z

src/matrixrtc/MembershipManager.ts

+/**
+ * Handles sending membership for MSC3401 RTC events.
+ */
+export class LegacyMembershipManager extends MembershipManager<SessionMembershipData> {


I recently read the style guide so 😇

There should be approximately one interface, class, or enum per file unless the file is named
"types.ts", "global.d.ts", or ends with "-types.ts".

BillCarsonFr · 2025-11-07T10:12:27Z

src/matrixrtc/MembershipManager.ts

+ * This exclusively sends RTCMembershipData
 */
-export class StickyEventMembershipManager extends MembershipManager {
+export class StickyEventMembershipManager extends MembershipManager<RtcMembershipData> {


I invoke the style guide rule number 30 ⚡️

Half-Shot added 3 commits October 27, 2025 12:45

Remove .sessionForRoom

f1e6fb7

Implement slots

5b12b75

Lots of updates

d4b90e3

Half-Shot mentioned this pull request Oct 28, 2025

Support new MatrixRTC implementations with Element Call element-hq/element-web#31088

Draft

9 tasks

Half-Shot added the T-Enhancement label Oct 28, 2025

Half-Shot added 2 commits October 30, 2025 12:00

Cleanup membership and set explicit boundaries on legacy v.s. new.

b0a6147

More refactors

4ca30be

toger5 reviewed Oct 30, 2025

View reviewed changes

Half-Shot added 7 commits November 3, 2025 12:07

Split membership manager into legacy variant to improve readability

3e5c1ee

more test fixes

addb9c0

Merge remote-tracking branch 'origin/develop' into hs/use-rtc-slots-f…

efd5874

…or-call

fix imports

f16760e

more test changes

7b39170

Cleanup tests

dcf7e87

More lint

c5fb489

github-actions bot deployed to PR Documentation Preview November 6, 2025 10:49 View deployment

Half-Shot added 2 commits November 6, 2025 11:54

fix circular imports

32761ea

fix lint

c56fe52

github-actions bot deployed to PR Documentation Preview November 6, 2025 13:26 View deployment

BillCarsonFr reviewed Nov 7, 2025

View reviewed changes

		@@ -0,0 +1,121 @@
		import { type IContent } from "../../matrix.ts";

Uh oh!

[MatrixRTC] Implement MSC4143 slots #5056

Are you sure you want to change the base?

[MatrixRTC] Implement MSC4143 slots #5056

Uh oh!

Conversation

Half-Shot commented Oct 28, 2025

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillCarsonFr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillCarsonFr Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BillCarsonFr Nov 7, 2025 •

edited

Loading